Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

vendor: upgrade gofakes3 to v0.0.11 #472

Merged
merged 6 commits into from
Jul 29, 2022
Merged

vendor: upgrade gofakes3 to v0.0.11 #472

merged 6 commits into from
Jul 29, 2022

Conversation

kucukaslan
Copy link
Contributor

@kucukaslan kucukaslan commented Jul 22, 2022 •
edited
Loading

Upgraded gofakes version to v0.0.11 to solve unaligned bit error in 32 bit architectures.
Allow disabling race flag (race flag is incompatible with -buildmode=pie)

Fixes #446

@kucukaslan
vendor: upgrade gofakes3 to v0.0.11
d15571c 
Upgraded gofakes version to v0.0.11 to solve unaligned bit error in 32 bit architectures.

Updates #446
@kucukaslan kucukaslan requested a review from a team as a code owner July 22, 2022 16:13
@kucukaslan kucukaslan requested review from igungor and ilkinulas and removed request for a team July 22, 2022 16:13
@kucukaslan
e2e: parse the flags, add "test.race.disabled" flag
96b27c2 
The flags of e2e package was not being parsed, so It always used the default values. To fix it,  it will parse the flags in TestMain method.

Add "test.race.disabled" flag to e2e. package to allow users to disable race flag.
Updates #446
@kucukaslan
Copy link
Contributor Author

kucukaslan commented Jul 26, 2022 •
edited
Loading

Warning
This approach is abandoned. See further comments -when available.

The commit 96b27c2 provides a way to solve the second problem of #446.

Using

go test -mod=vendor -count=1 ${RACE_FLAG} `go list ./... | grep -v e2e`
go test -mod=vendor -count=1 ${RACE_FLAG} ./e2e -args -test.race.disabled

instead of go test -mod=vendor -count=1 ${RACE_FLAG} ./... will disable race flag in the e2e package tests.

e2e package is treated differently because only it has test.race.disabled flag. Alternatively, we can add this flag to all packages (to those have tests). But I prefered not to add unused flags to them just to allow writing go test -mod=vendor -count=1 ${RACE_FLAG} ./e2e -args -test.race.disabled.

@kucukaslan
makefile: use different test targets to disable race flag in tests.
f3aa6a9 
Since windows should not use race flag change the ci.yml accordingly.
@kucukaslan
Copy link
Contributor Author

kucukaslan commented Jul 26, 2022 •
edited
Loading

With 96b27c2, e2e package request an environment variable (named S5CMD_TEST_DISABLE_RACE) to disable the race flag when compiling the test binary.
To handle both cases we added a new Makefile target named test_without_race to be used in the Windows and other distributions that defaults go's buildmode to PIE.

So 2nd problem of #446 will be solved by using make test_without_race instead of make test.

  • Add test_with_race target

Note:
An undefined flag will give error, there is no way to ignore the errors and there won't be.

igungor
igungor reviewed Jul 28, 2022
View reviewed changes
Makefile Outdated Show resolved Hide resolved
igungor
igungor reviewed Jul 28, 2022
View reviewed changes
e2e/main_test.go Outdated Show resolved Hide resolved
igungor
igungor reviewed Jul 28, 2022
View reviewed changes
e2e/util_test.go Outdated Show resolved Hide resolved
@ilkinulas ilkinulas merged commit 123b1c7 into peak:master Jul 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@igungor igungor igungor approved these changes

@ilkinulas ilkinulas ilkinulas approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Tests failed on AlpineLinux 32bit architecture
3 participants
@kucukaslan @igungor @ilkinulas